Add component container for EventsCBGExecutor + Isolated component container for all events based executors - #3123
Conversation
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
Signed-off-by: Skyler Medeiros <skye@polymathrobotics.com>
| if (executor_type == "events") { | ||
| using executor = rclcpp::experimental::executors::EventsExecutor; | ||
| using ComponentManagerIsolated = rclcpp_components::ComponentManagerIsolated<executor>; | ||
| exec = std::make_shared<executor>(); |
There was a problem hiding this comment.
Personally I don't think anything in the experimental namespace should be officially supported in a thing like this. Especially knowing that this is going to be depreciated in short order with the new one
There was a problem hiding this comment.
probably we can say EventExecutor is now graduated phase as official for next turtle-M release from the incubation experimental phase?
| } else { | ||
| using ComponentManagerIsolated = | ||
| rclcpp_components::ComponentManagerIsolated<rclcpp::executors::SingleThreadedExecutor>; | ||
| } else if (executor_type == "cbg-single-threaded") { |
There was a problem hiding this comment.
I think there might be an issue here. Taking the SingleThreadedExecutor and MultiThreadedExecutor as examples, we currently always create a SingleThreaded Executor to manage the components, and pass the executor type to the isolated component manager based on the cli args. The SingleThreadedExecutor is responsible for loading and unloading components
If I am understanding this correctly, with this PR we are creating a single/threaded event executor based on the cli args to manage the components, but then for each component we always create a multi-threaded event executor, correct?
I also wonder if we really need separate cbg-single-threaded vs cbg-multi-threaded. Instead, I think we could take the thread count from ros arguments (similar to the new executable in this PR), and if it is not specified, default to the multi-threaded event executor. What do you think?
There was a problem hiding this comment.
good callout. I'll make those changes on the other PR
|
Closed in favor of #3134 |
Enable use_intra_process_comms(true) for the mavros_node container and the dynamically created plugin nodes, so the internal MAVLink bus and co-located subscribers go zero-copy (cross-process consumers fall back to DDS, so this is safe either way). test_compose.launch.py: - fcu_url/gcs_url as launch arguments instead of hardcoded URLs - add an 'executor' argument (mt by default, events/auto opt-in). The Callback Group Events executor's component-container support is still immature upstream (ros2/rclcpp#3186: shutdown double-remove; ros2/rclcpp#3123: container support), so default to the reliable component_container_mt and only use component_container --executor-type events-cbg when asked - resolve URLs in an OpaqueFunction so list parameters keep their type - tone down container log level from DEBUG to INFO
Enable use_intra_process_comms(true) for the mavros_node container and the dynamically created plugin nodes, so the internal MAVLink bus and co-located subscribers go zero-copy (cross-process consumers fall back to DDS, so this is safe either way). test_compose.launch.py: - fcu_url/gcs_url as launch arguments instead of hardcoded URLs - add an 'executor' argument (mt by default, events/auto opt-in). The Callback Group Events executor's component-container support is still immature upstream (ros2/rclcpp#3186: shutdown double-remove; ros2/rclcpp#3123: container support), so default to the reliable component_container_mt and only use component_container --executor-type events-cbg when asked - resolve URLs in an OpaqueFunction so list parameters keep their type - tone down container log level from DEBUG to INFO Signed-off-by: Vladimir Ermakov <vooon341@gmail.com>
Enable use_intra_process_comms(true) for the mavros_node container and the dynamically created plugin nodes, so the internal MAVLink bus and co-located subscribers go zero-copy (cross-process consumers fall back to DDS, so this is safe either way). test_compose.launch.py: - fcu_url/gcs_url as launch arguments instead of hardcoded URLs - add an 'executor' argument (mt by default, events/auto opt-in). The Callback Group Events executor's component-container support is still immature upstream (ros2/rclcpp#3186: shutdown double-remove; ros2/rclcpp#3123: container support), so default to the reliable component_container_mt and only use component_container --executor-type events-cbg when asked - resolve URLs in an OpaqueFunction so list parameters keep their type - tone down container log level from DEBUG to INFO Signed-off-by: Vladimir Ermakov <vooon341@gmail.com>
4 participants
Description
Fixes # (issue)
Is this user-facing behavior change?
Yes, users will now be able to use the EventsCBGExecutor in a component container, and both the EventsExecutor and EventsCBGExecutor in an isolated component container.
Did you use Generative AI?
Nope
Additional Information
Depends on #3097
Partial re-implementation of #2541, but with additional options for the EventsCBGExecutor to be run in single threaded or multithreaded mode